Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DXCDT-255: Add new email provider manager and deprecate old one #129

Merged
merged 9 commits into from
Nov 7, 2022

Conversation

sergiught
Copy link
Contributor

@sergiught sergiught commented Nov 4, 2022

🔧 Changes

Support for email provider settings has been requested through auth0/terraform-provider-auth0#384.

As we started adding this setting to the old Email struct, it was noted that the DX wasn't good enough to be able to distinguish between what settings and even what credentials are specific to each email provider.

To solve that we're introducing a brand new EmailProvider struct with specific credentials and settings for each email provider type.

Also to avoid doing breaking changes we're just marking the old Email as deprecated and it will be removed in a future version.

📚 References

🔬 Testing

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@sergiught sergiught self-assigned this Nov 4, 2022
@sergiught sergiught marked this pull request as ready for review November 4, 2022 07:05
@sergiught sergiught requested a review from a team as a code owner November 4, 2022 07:05
@codecov-commenter
Copy link

codecov-commenter commented Nov 4, 2022

Codecov Report

Base: 95.39% // Head: 95.24% // Decreases project coverage by -0.15% ⚠️

Coverage data is based on head (f9ace1c) compared to base (fe3727a).
Patch coverage: 91.11% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #129      +/-   ##
==========================================
- Coverage   95.39%   95.24%   -0.16%     
==========================================
  Files          37       38       +1     
  Lines        6085     6310     +225     
==========================================
+ Hits         5805     6010     +205     
- Misses        226      241      +15     
- Partials       54       59       +5     
Impacted Files Coverage Δ
management/email.go 100.00% <ø> (ø)
management/email_provider.go 74.35% <74.35%> (ø)
management/management.gen.go 100.00% <100.00%> (ø)
management/management.go 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -19,6 +20,7 @@ type Email struct {
}

// EmailCredentials are used for authenticating Email Providers.
// Deprecated: Use EmailProvider instead.
type EmailCredentials struct {
// API User
APIUser *string `json:"api_user,omitempty"`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To note that we're not adding the APIUser field to any of the credentials type within the new EmailProvider struct. This is because this field is no longer supported and trying to include it in the payload will result in a 400:

{
  "statusCode": 400,
  "error": "Bad Request",
  "message": "Payload validation error: 'None of the valid schemas were met' on property credentials (Credentials required to use the provider). Inner errors: [ Payload validation error: 'Additional properties not allowed: api_user' on property credentials ({description}). ].",
  "errorCode": "invalid_body"
}

@sergiught sergiught requested a review from willvedd November 4, 2022 16:53
@sergiught sergiught requested a review from Widcket November 5, 2022 08:10
@sergiught sergiught merged commit 46a47a9 into main Nov 7, 2022
@sergiught sergiught deleted the feature/DXCDT-255-email-provider-settings branch November 7, 2022 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants